fix(core): keep stdout pure when -o json/yaml/csv is requested - #99
Closed
GregHolmes wants to merge 1 commit into
Closed
fix(core): keep stdout pure when -o json/yaml/csv is requested#99GregHolmes wants to merge 1 commit into
GregHolmes wants to merge 1 commit into
Conversation
Closes #98. Commands print status lines and human tables for people, while the framework writes the serialised result to stdout. With a machine-readable format those two collided on the same stream, so the JSON arrived after the human output: $ dg -o json projects | jq . Fetching projects... <- stdout Found 1 project(s): <- stdout { ... } <- stdout, too late for jq The payload was never missing, just buried — in `dg -o json models` the JSON started 37KB into stdout, behind a Rich table. `listen` and `speak` were unaffected because they gate their own display on the format; the other eight commands did not. Status routing previously keyed off `agentic` (a TTY/env heuristic) rather than "does something else own stdout", so an explicit `-o json` on a terminal still sent status to stdout. - Add `StatusConsole`, which resolves its target at write time: stderr when the active format is in MACHINE_FORMATS (json/yaml/csv), stdout otherwise. The shared `console` becomes one, so print_info/success/warning/panel/separator follow automatically. - Add `stdout_console` for the payload and point `print_output` at it, so the result is never diverted along with the status text. - Add `get_status_console()` and use it in read, models, projects, keys, billing, usage, requests and members in place of a private `Console()`. `table` is deliberately not a machine format: it is a human rendering and keeps stdout. Deliberately avoided redirecting sys.stdout around handle(): `dg speak` streams audio via `sys.stdout.buffer.write()` and branches on `sys.stdout.isatty()`, so a blanket redirect would corrupt piped audio. Verified: stdout is parseable JSON for all eight commands (plus valid YAML/CSV); `dg speak | ffplay` still receives a RIFF/WAV stream with no JSON mixed in; `dg -o json listen | jq` unchanged; human TTY output byte-for-byte as before, with status now on stderr under -o json. 1059 unit tests pass (7 new regression tests for the routing) and the 68-check live API smoke suite is green.
Contributor
Author
|
Superseded by #101 — closing. #97 landed the per-command pattern ( The idea from this PR worth keeping — a shared primitive rather than a |
GregHolmes
added a commit
that referenced
this pull request
Aug 18, 2026
…sweep) (#101) Last command in the `-o json` sweep started by #97, plus the review follow-ups that came out of it. #97 fixed **requests, read, models, projects, members, usage, billing** — but `keys` wasn't in that sweep, so it was left as the one account command whose stdout still broke pipes: ```console $ dg -o json keys | jq -r '.keys[0].key_id' jq: parse error: Invalid numeric literal at line 1, column 9 ``` `Fetching API keys...` and the Rich table were going to stdout ahead of the JSON. ## The keys fix — same pattern as the merged seven - `status_console` for progress, errors and the empty-state notice, so chrome never touches stdout - Human rendering gated on `get_output_format() == "default"`: the list table, created-key details, key details, and both dry-run summaries The created key's **secret is unaffected** — it already travels in `KeysResult.created_key.key`, so json/yaml/csv callers still receive it; only the human echo is suppressed. There's a test asserting exactly that. ## Review follow-ups Reviewing the above turned up four more defects. All predate this branch, but three of them sit in code it touches and all four make "`-o json` keeps stdout parseable" true only in the narrow case, so they're fixed here rather than deferred. **`dg keys --create --dry-run` never ran.** `handle` read `project_id` and `dry_run` with `.get()` and then forwarded `**kwargs` alongside them, so every argument arrived twice and Python raised `got multiple values for argument 'project_id'` before the body started — swallowed into an error result, exit 0. The dry-run gating added here was unreachable, and this PR's original "known limitation" note described behaviour that path never had. Popping both fixes it; fixing only `dry_run` just exposes the `project_id` collision behind it. **Failures exited 0.** `main.py` caught `SystemExit` and discarded the code, so no command could ever signal failure — including the auth-guard failure that already raised `SystemExit(1)`. `if dg -o json keys; then` took the success branch on a failed call. The code is now carried through the post-run notifications and re-raised, and `BaseCommand` maps status to exit code in one table, matching the contract already published in `llms-full.txt` (`0 = success, 1 = error, 2 = user interrupt`): `error` → 1, `cancelled` → 2, everything else → 0. Exiting 0 on failure violated that documented contract, so this restores published behaviour rather than introducing a new one — no docs change needed. **`-o yaml` and `-o csv` silently deleted user data.** Both printed through Rich, which treats square brackets as style markup and removes them — a key comment of `[ci] runner` came out as `runner`, no error. Rich also hard-wrapped at the console width, injecting newlines into the middle of a csv field. Both now write the payload verbatim (markup, highlighting and wrapping off), still via `console` so `--quiet` keeps working. The JSON path was already safe (`print_json` escapes rather than interprets) and is untouched. **`dg keys --delete ID` without `--yes` deleted nothing and blamed the user.** `BaseCommand.confirm` returns its default whenever any parameter came from the command line, and `--delete KEY_ID` is itself such a parameter — so the prompt was unreachable and the command always returned `Cancelled by user` without asking or calling the API. It now prompts on stderr when someone is there to answer (stderr, so `-o json` stdout stays clean), and returns a usage error naming `--yes` when nobody is. Also: `get_status_console()` in core replaces the eight per-command `Console(stderr=True)` declarations. A per-command console silently missed core's agentic no-color settings, and a new command reaching for a bare `Console()` is exactly how `keys` regressed in the first place — this makes it correct by default. ## Verification | check | result | |---|---| | `dg -o json keys \| jq -r '.keys[0].key_id'` | returns the id ✓ (was a parse error) | | all 8 commands under `-o json` | keys, projects, models, usage, requests, members, billing, read — all valid JSON on stdout ✓ (models is 77KB, so the wrapping fix holds at size) | | `-o yaml` / `-o csv` for keys | valid, and bracketed values now survive verbatim ✓ | | chrome under `-o json` | still on stderr ✓ | | human output | unchanged under a pty ✓ | | `keys --create --dry-run` | reports `dry_run`, calls nothing ✓ (was a TypeError) | | `keys --delete ID` without `--yes` | usage error naming `--yes`, exit 1, no API call ✓ | | exit codes | error → 1, cancelled → 2, success/dry_run/`--help`/`--version` → 0, unknown command → 2 ✓ — matches documented contract | | tests | **1089 passed** (22 new), `ruff check` and `mypy` clean | ## Still open in #98 **#98 stays open for one item:** the promise that stdout auto-switches to JSON *when piped*. `dg keys | jq` still gets the Rich table, because the auto-switch (`setup_output`) sits behind `is_agentic()`, which needs 3+ soft signals; a plain pipe from an interactive shell scores 1. Lowering that threshold changes what `dg <anything> | less` does for every command, so it's a product decision rather than a bug fix — deliberately not made here. ## Note on #99 #99 fixed the stdout bug with a central format-aware console before #97 landed. Now that #97 is merged, #99 is redundant and would conflict, so I've closed it. Its one good idea — a shared status-console primitive instead of per-command declarations — is implemented here as `get_status_console()`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #98. Branched off
main, independent of the other open PRs.The bug
Commands print status lines and human tables for people; the framework writes the serialised result to stdout. Under a machine-readable format both went to the same stream:
The payload was never missing — just buried. In
dg -o json modelsthe JSON started 37 KB into stdout, behind a Rich table.Status routing keyed off
agentic(a TTY/env heuristic) rather than "does something else own stdout", so an explicit-o jsonon a terminal still sent status to stdout.listenandspeakescaped because they gate their own display on the format; the other eight commands didn't.The fix
StatusConsoleresolves its write target at write time: stderr when the active format is inMACHINE_FORMATS(json/yaml/csv), stdout otherwise. The sharedconsoleis one, soprint_info/print_success/print_warning/print_panel/print_separatorfollow automatically.stdout_consolecarries the payload;print_outputnow uses it, so the result is never diverted along with the status text.get_status_console()replaces the privateConsole()in the eight affected commands — a one-line change each.tableis deliberately not a machine format: it's a human rendering and keeps stdout.What I deliberately did not do
Wrapping
handle()inredirect_stdoutwould have been a smaller diff, butdg speakstreams audio throughsys.stdout.buffer.write()and branches onsys.stdout.isatty()— a blanket redirect would corrupt piped audio and flip that branch. Routing at the console layer leavessys.stdoutuntouched.Verification
dg -o json {models,projects,keys,usage,billing,requests,members,read}-o yaml/-o csvdg speak "..." | ffplaydg -o json listen f.mp3 | jq-o jsonin a ptyThe 5
print_outputtests that asserted it writes viaconsolewere repointed tostdout_console— that coupling is precisely what this change fixes.Note for review
Three files (
output.py,projects/command.py,usage/command.py) are stored with CRLF line endings. My first pass normalised them to LF and ballooned the diff to ~1300 lines; I restored their original convention, so the diff is now 127 insertions / 31 deletions. Worth considering a.gitattributesto settle line endings repo-wide — mixed conventions make diffs like that easy to create by accident.